-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: add new sphinx-external-toc package #956
Conversation
6bc4245
to
d86b0d9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really nice! Didn't totally parse all the coercion logic, but it looks sensible. And the JSON validation from types is good - easy!
const ajv = new Ajv.default(); | ||
const validate = ajv.compile(schema); | ||
if (!validate(toc)) { | ||
throw new Error( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worth adding validate.errors
to this message? I think you were saying those errors were sometimes totally undecipherable because of all the complexity with unions, so probably not, I suppose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure -- as you note, I found that the errors are fairly non-intuitive. I think for now it might be sufficient just to complain about an invalid TOC.
I was just wondering if this should be a separate package on executablebooks, since it isn't really MyST related...? |
That's a good question. I think for now we keep it here because it will exclusively be used by myst-cli. Soon it will end up in jupyterlab-myst / jupyter-book, and at that time we can pull it out once the organisational structure is clearer? |
Co-authored-by: Franklin Koch <franklinwkoch@gmail.com>
3d8b23e
to
1ec13bb
Compare
@agoose77 looks like test are failing here. Are you working on that to get this ready to review? |
Typical! I just rebased it, so I'll check out what's regressed. I think it's to do with AJV. c.f. ajv-validator/ajv#2389 |
I understand the TOC round-tripping issues a bit better. Thanks for the call @agoose77. Plan we discussed:
|
This is no longer necessary, as we are using our own TOC |
This PR adds a new
sphinx-external-toc
package. The intention is to replicate the functionality of thesphinx-external-toc
Python package, for later plumbing into MyST-CLI and JupyterLab-MyST.This PR: